-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
eslint plugin no-undeclared-env-vars should detect known framework variables #9110
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
8 Skipped Deployments
|
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
Hi there 👋 It looks like this PR introduces broken links to the docs, please take a moment to fix them before merging:
Thank you 🙏 |
492e835
to
7cdc458
Compare
32efb97
to
309735d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! A few suggestions and I want to test it out locally but overall looking great.
packages/eslint-plugin-turbo/lib/rules/no-undeclared-env-vars.ts
Outdated
Show resolved
Hide resolved
packages/eslint-plugin-turbo/lib/rules/no-undeclared-env-vars.ts
Outdated
Show resolved
Hide resolved
packages/eslint-plugin-turbo/__fixtures__/framework-inference/package.json
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last thing to fix up that should fix the tests, and then let's ship it!
packages/eslint-plugin-turbo/lib/rules/no-undeclared-env-vars.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving so you can merge whenever the tests are good!
Co-authored-by: Thomas Knickman <[email protected]>
0fbcc95
to
73ba593
Compare
Very excited for this. Thanks, Dimitri! |
Description
This PR picks up from #8505
We currently do Framework Inference based on the package.json dependencies of the nearest package.json. turbo's Framework Inference support is per-package whereas this one wholesale allowslists all framework environment variables in every package.
The job of this PR, therefore, is to enable per-package framework detection.
I started by extracting this data to a json file, frameworks.json, so it can be shared between Rust and TypeScript.
TODOs:
framework.rs
intoframeworks.json
frameworks.json
file to generate the table inusing-environment-variables.mdx
Testing Instructions
After building the plugin (which will run it locally), open up
packages/eslint-plugin-turbo/__fixtures__/framework-inference/apps/vite
. Notice that there are no errors inindex.js
, but if you remove thevite
dependency inpackage.json
then there will be. Notice, also, that adding a next.js env var in this file (e.g.process.env.NEXT_PUBLIC_ZILTOID
) will error as expected.